Test Results

   285 files     285 suites   48m 59s ⏱️
 3 586 tests  3 510 ✅  76 💤 0 ❌
10 950 runs  10 719 ✅ 231 💤 0 ❌

Results for commit aa6196f.

♻️ This comment has been updated with latest results.

I think this logic assumes that metadata repositories and artifact repositories are always colocated.

Target locations always assume that, I'm not aware one is able to specify different ones (and have never really seen this), also PDE assume this on multiple places.

Does this logic really do that?

The logic asks the repository manager to update all repositories (maybe referenced ones too) to refresh, if that "do not work" then this has to be considered at P2/Manger level and not at PDE/Client side.

In any case, would not a flushCache be far simpler and accomplish the same thing (yes perhaps flushing more than necessary, but also not missing anything) without assuming the colocation of metadata and artifact?

flushCache is not an API (maybe for good reasons) and this will as you mentioned flush much more, e.g assume I update one target with one location it will result that immediately all other update sites need to be reloaded (what could be many more). Beside that a cache-flush do not mean it is really reloaded because refresh does much more including some events and enforce download the data from the server.

So either we should drop the caching at all or do it selectively, it does not make sense to cache something when on the other hand one then needs hacks to flush the cache unconditionally.

I have a bad feeling that things will be missed. E.g., when a composite is loaded, that can load a whole whack of children and any of those children my need to be refreshed, and so on recursively.

If I look at the source I see this:

		try {
			IRepository<T> result = loadRepository(location, monitor, null, 0);
			loaded = true;
			setEnabled(location, wasEnabled);
			if (result instanceof ICompositeRepository<?>) {
				for (URI childLocation : ((ICompositeRepository<?>) result).getChildren()) {
					basicRefreshRepository(childLocation, monitor);
				}
			}
			return result;
		}

so composites are refreshed recursively already

If a composites refresh recursively then that's a good thing! I think non-collocated metadata and artifact data is rare enough we could just ignore that detail. So this approach looks reasonable.

If no one objects I want to merge this so we have enough time for finding any issues and base further changes on top of this.